fix(v2): await run log finalization before sync execute responds by lakshya-dhariwal · Pull Request #8369 · simstudioai/sim

lakshya-dhariwal · 2026-09-28T07:56:05Z

Summary

Fixes #8354. The v2 sync execute route could return completed while the run's log row was still being finalized: executeWorkflowCore defers status/endedAt/cost persistence into the logging session's post-execution promise, and the sync path in executeWorkflowService returned without awaiting it. A GET /api/v2/logs/{runId} right after the response could still show status: "running", a null endedAt, and missing cost items.

The sync response now waits for the post-execution promise to settle before it is returned, so a terminal response doubles as a durable receipt. Async (queued) runs are unaffected: they already return before execution starts.

Type of Change

Testing

New route test holds the sync response until post-execution logging finalizes: gates waitForPostExecution on a manually released promise and asserts the response is not sent before release. Verified it fails without the fix and passes with it.
vitest run app/api/v2/workflows/[workflowId]/execute/route.test.ts: 39/39 pass.
biome check on the changed files: clean.
Reviewers can focus on the finally in runSynchronousWorkflow (apps/sim/lib/workflows/executor/execute-service.ts): the await covers both the success and error terminal results, and is a no-op whenever the core never set a post-execution promise.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

N/A - server-side API behavior change, covered by the new test.

vercel · 2026-09-28T07:56:12Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Sep 28, 2026 8:18am UTC

greptile-apps · 2026-09-28T07:58:54Z

Confidence Score: 4/5

[Medium risk] Workflow execution endpoint now waits for logging to finish.

The response-ordering change appears sound, but the test must satisfy the repository’s shared-utility requirement before merging.

Findings

Finalization wait has no deadline ▶
Test can miss its gate ▶
Inline delay violates shared-utility rule ▶

Summary

The PR makes synchronous v2 workflow execution wait for post-execution logging before returning its terminal result and adds a route-level regression test.

The new wait has no deadline after execution-timeout and request-abort cleanup.
The test’s fixed delay can race with entry to its mocked finalization gate and uses a timeout pattern prohibited by a repository instruction.

Diagram

sequenceDiagram
  participant Client
  participant Route
  participant Service
  participant Core
  participant Logs
  Client->>Route: POST sync execute
  Route->>Service: executeWorkflowService
  Service->>Core: executeWorkflowCore
  Core-->>Service: Execution result
  Core->>Logs: Start post-execution finalization
  Service->>Logs: waitForPostExecution
  Logs-->>Service: Finalization settles
  Service-->>Route: Terminal result
  Route-->>Client: Sync response

_{Reviews (1) · Last reviewed commit: "fix(v2): await run log finalization befo..."}

greptile-apps · 2026-09-28T07:58:57Z

+        // the log (status, endedAt) and cost ledger as soon as it lands. The
+        // core defers that finalization into the session's post-execution
+        // promise, so hold the response until the log is final.
+        await loggingSession.waitForPostExecution()


Finalization wait has no deadline If a finalization database write stalls, the sync response can remain open indefinitely. This wait runs after the execution timeout is cleaned up and the request-abort listener is removed, so neither can stop it. Consider bounding the wait or stopping it when the request is no longer live.

Knowledge Base Used: Application platform

Not taking this one. Bounding the wait would reintroduce #8354 through the back door: if the deadline fires first, the response returns completed while the log row is still unfinalized, which is the exact race this PR removes. The same unbounded await already exists on the v1 path (execute-workflow.ts awaits waitForPostExecution() in its own finally), so this matches the established pattern. A genuinely stalled finalization write now surfaces as a slow request instead of a log row stuck on running forever, which is the more debuggable failure mode.

That’s a fair correction. Given the sync endpoint’s terminal-receipt contract, bounding this await would indeed recreate the original race whenever finalization outlives the deadline. The v1 path already uses the same unbounded waitForPostExecution() in its finally, so matching that behavior is consistent. A slow finalization delaying the response is preferable to acknowledging a run whose durable log is still running; I’m withdrawing this concern.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

greptile-apps · 2026-09-28T07:58:57Z

+    await new Promise((resolve) => setTimeout(resolve, 10))
+    expect(responded).toBe(false)
+
+    releaseFinalization()


Test can miss its gate The fixed 10 ms delay does not establish that execution reached the finalization gate. If the mocked route takes longer, the assertion passes without testing the gate and releaseFinalization() then fails because it has not been assigned. Wait for entry to the gate before asserting or releasing it.

Good catch, fixed in 5d98041. The test now waits on vi.waitFor for waitForPostExecution to be entered before asserting the response is held, so it can no longer pass without reaching the gate.

greptile-apps · 2026-09-28T07:58:57Z

+    })
+
+    // Let the mocked execution settle up to the finalization gate.
+    await new Promise((resolve) => setTimeout(resolve, 10))


Inline delay violates shared-utility rule This test implements a delay with new Promise and setTimeout, but the repository requires the shared sleep(ms) helper from @sim/utils/helpers instead. If a timed delay remains after the test is synchronized, replace it with that helper before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Resolved by the same commit - the inline setTimeout is gone. The test synchronizes on vi.waitFor against the mock instead of a timed delay.

The v2 sync execute route returned 'completed' while the run log was still being finalized: execution-core defers status/endedAt/cost persistence into the logging session's post-execution promise, and nothing on the service's sync path awaited it. A GET on the run's log right after the response could still show status 'running', a null endedAt, and missing cost items. Hold the sync response until the post-execution promise settles, so the terminal response doubles as a durable receipt. Async (queued) runs are unaffected; they already return before execution starts. fixes simstudioai#8354

vercel Bot temporarily deployed to Preview September 28, 2026 07:56 Inactive

greptile-apps Bot reviewed Sep 28, 2026

View reviewed changes

lakshya-dhariwal force-pushed the fix/v2-sync-execute-await-log-finalization branch from dfd7f03 to 5d98041 Compare September 28, 2026 08:18

vercel Bot temporarily deployed to Preview September 28, 2026 08:18 Inactive

This was referenced Sep 30, 2026

fix(execution): publish a pause only after its run log is finalized #8446

Merged

fix(logs): record a run's cost before it reads finished, and hold sync execute responses until the log is final #8473

Merged

Navigation Menu

fix(v2): await run log finalization before sync execute responds - #8369

fix(v2): await run log finalization before sync execute responds#8369
lakshya-dhariwal wants to merge 1 commit into
simstudioai:stagingfrom
lakshya-dhariwal:fix/v2-sync-execute-await-log-finalization

lakshya-dhariwal commented Sep 28, 2026

Uh oh!

vercel Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

lakshya-dhariwal Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

lakshya-dhariwal Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

lakshya-dhariwal Sep 28, 2026

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lakshya-dhariwal commented Sep 28, 2026

Summary

Type of Change

Testing

Checklist

Screenshots/Videos

Uh oh!

vercel Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

lakshya-dhariwal Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

lakshya-dhariwal Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

lakshya-dhariwal Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Sep 28, 2026 •

edited

Loading

greptile-apps Bot commented Sep 28, 2026 •

edited

Loading